[Prototype] ScyllaDB#3422
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3422 +/- ##
==========================================
- Coverage 59.25% 59.14% -0.12%
==========================================
Files 2110 2117 +7
Lines 174181 175478 +1297
==========================================
+ Hits 103210 103779 +569
- Misses 62044 62724 +680
- Partials 8927 8975 +48
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| func scyllaHistoricalOffloadConfigured(cfg config.StateStoreConfig) bool { | ||
| return strings.TrimSpace(cfg.HistoricalOffloadScyllaHosts) != "" || | ||
| strings.TrimSpace(cfg.HistoricalOffloadScyllaKeyspace) != "" |
There was a problem hiding this comment.
Partial Scylla config crashes node at startup
Medium Severity
scyllaHistoricalOffloadConfigured uses || (OR), so setting only HistoricalOffloadScyllaKeyspace without hosts (or vice versa) makes the function return true. The code then attempts to open a Scylla session with incomplete config, which fails validation and crashes node startup. Using && would be safer since both hosts and keyspace are required for a valid connection — a partial config would simply be treated as "not configured" rather than causing a startup failure.
Reviewed by Cursor Bugbot for commit 5ef70ba. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4a14131. Configure here.
| } | ||
| } | ||
| return g.Wait() | ||
| } |
There was a problem hiding this comment.
Internal errgroup cancellation misidentified as external cancellation
Medium Severity
In writeRecordsPipelined, all record-row goroutines share gctx from an errgroup. When a later record's write fails, the errgroup cancels gctx, causing earlier in-flight goroutines to fail with context.Canceled. Since rowDone channels are read in order, the main loop surfaces the earlier record's cancellation error. writeBatchWithRetry then calls isCancellation on this error, treats it as a real cancellation, and skips retries. workerLoop also sees the cancellation and silently returns nil, killing the worker. The fetcher eventually blocks trying to send to the dead worker's full shard channel, stalling the entire consumer.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 4a14131. Configure here.
1 participant
Describe your changes and provide context
Testing performed to validate your change
Note
High Risk
High risk because it changes the state-store construction and introduces optional network-backed fallback reads for pruned historical versions, plus new Kafka→Scylla ingestion code paths that can affect correctness and operational reliability when enabled.
Overview
Adds a prototype ScyllaDB/Cassandra-backed historical state offload.
Nodes can now be configured (new
state-store.historical-offload-scylla-*flags and TOML fields) to wrap the primary SS with ahistorical.FallbackStateStorethat falls back to Scylla/Cassandra for pruned point reads (Get/Has) while keeping iteration/writes on local SS, with an in-memory LRU cache for repeated lookups.Introduces a new Kafka consumer binary (
historical-scylla-consumer) plus Scylla schema and sink implementation to ingest SS changelog entries intostate_mutations/state_versions, and exportsoffload.NewSASLMechanismso the consumer shares the producer’s Kafka auth configuration. Also adds thegocqldependency (and related indirect deps) and updates tests to cover the new config and sink/reader behavior.Reviewed by Cursor Bugbot for commit 4a14131. Bugbot is set up for automated code reviews on this repo. Configure here.